-
Notifications
You must be signed in to change notification settings - Fork 37
Deployment Feature #671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Deployment Feature #671
Conversation
WalkthroughThe pull request transitions the platform from API revision-based deployments to immutable deployment artifacts. It removes revision tracking fields, introduces deployment status management with unique deployment identifiers, adds comprehensive lifecycle endpoints (deploy, redeploy, undeploy, delete), restructures the database schema, and creates new service and handler layers for deployment orchestration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant DeploymentHandler as Deployment Handler
participant DeploymentService as Deployment Service
participant APIRepo as API Repository
participant DBStore as Database
participant GatewayEventsService as Gateway Events Service
Client->>DeploymentHandler: POST /apis/{id}/deploy (DeployAPIRequest)
DeploymentHandler->>DeploymentService: DeployAPI(apiUUID, request, orgUUID)
DeploymentService->>APIRepo: Validate API exists
APIRepo->>DBStore: Query API metadata
DBStore-->>APIRepo: API record
APIRepo-->>DeploymentService: API found
DeploymentService->>APIRepo: CountDeploymentsByAPIAndGateway()
APIRepo->>DBStore: Count active deployments
DBStore-->>APIRepo: Count result
APIRepo-->>DeploymentService: Deployment count
alt Limit Exceeded
DeploymentService->>APIRepo: GetOldestUndeployedDeploymentByGateway()
APIRepo->>DBStore: Query oldest undeployed
DBStore-->>APIRepo: Old deployment record
APIRepo-->>DeploymentService: Old deployment
DeploymentService->>APIRepo: DeleteDeployment(oldDeploymentID)
APIRepo->>DBStore: Delete old deployment
end
DeploymentService->>APIRepo: CreateDeployment(newDeployment)
APIRepo->>DBStore: INSERT deployment with content/metadata
DBStore-->>APIRepo: Deployment created
APIRepo-->>DeploymentService: Success
DeploymentService->>APIRepo: GetActiveDeploymentByGateway()
APIRepo->>DBStore: Query existing active deployment
DBStore-->>APIRepo: Active deployment (if exists)
APIRepo-->>DeploymentService: Previous deployment
alt Previous Deployment Exists
DeploymentService->>APIRepo: UpdateDeploymentStatus(prev, 'UNDEPLOYED')
APIRepo->>DBStore: UPDATE status to UNDEPLOYED
DBStore-->>APIRepo: Updated
DeploymentService->>GatewayEventsService: BroadcastUndeploymentEvent()
GatewayEventsService->>GatewayEventsService: Serialize & broadcast
end
DeploymentService->>GatewayEventsService: BroadcastDeploymentEvent(newDeployment)
GatewayEventsService->>GatewayEventsService: Serialize & broadcast to gateways
DeploymentService-->>DeploymentHandler: DeploymentResponse
DeploymentHandler-->>Client: 201 Created {deploymentId, status, ...}
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/service/gateway_internal.go (1)
273-283: Set deployment Content and Status before insertThe
api_deploymentstable requirescontentasNOT NULLandstatusmust have a valid value. The current code creates anAPIDeploymentwith onlyApiID,GatewayID,OrganizationID, andCreatedAt—leavingContentandStatusunset. TheCreateDeploymentmethod will attempt to insert these empty/null values, causing the insert to fail. PopulateContentwith the API definition and setStatusto a valid value (e.g., "DEPLOYED") before callingCreateDeployment.
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/database/schema.sql`:
- Around line 220-236: The CREATE TABLE for api_deployments has a missing comma
between the table constraints UNIQUE(api_uuid, gateway_uuid, deployment_id,
organization_uuid) and CHECK (status IN (...)), which causes a SQL syntax error;
add a comma after the UNIQUE(...) constraint in the api_deployments table
definition so the UNIQUE constraint and the CHECK constraint are properly
separated and the table (functionally api_deployments) can be created.
In `@platform-api/src/internal/handler/deployment.go`:
- Around line 188-230: The DeleteDeployment handler currently maps service
errors to 500; update error handling after calling
deploymentService.DeleteDeploymentByHandle(apiId, deploymentId, orgId) to map
the service error that indicates an active deployment (e.g.,
constants.ErrDeploymentActive or the specific error returned by your service) to
HTTP 409 with an appropriate utils.NewErrorResponse(409, "Conflict", "<human
message>"), leaving other errors as-is; also return no body for success by using
c.Status(http.StatusNoContent) (or c.JSON(http.StatusNoContent, nil)
consistently) instead of sending a body with 204.
- Around line 256-271: The 500 response currently returns "Deployment not found"
which masks internal errors; in the error branch after calling
h.deploymentService.GetDeploymentByHandle (and after checking
constants.ErrAPINotFound and constants.ErrDeploymentNotFound) change the c.JSON
internal server error body to include the actual error details (e.g.,
err.Error() or a message like "Failed to retrieve deployment") instead of
"Deployment not found" so the response accurately reflects the internal failure
while keeping the existing log.Printf call.
- Around line 314-355: In GetDeploymentContent, the handler currently falls
through after sending error JSON (causing a second response) and sends the wrong
media type; add a return immediately after each c.JSON error response in the
error branch of h.deploymentService.GetDeploymentContentByHandle so the function
exits on error, and change the final c.Data content type from "application/json"
to the correct YAML media type (e.g., "application/x-yaml" or "text/yaml") to
match GenerateAPIDeploymentYAML-produced artifacts.
- Around line 144-186: Handler UndeployDeployment currently sends a 204 with a
JSON null (c.JSON(http.StatusNoContent, nil)) which mismatches the API contract;
update the handler to either return the spec'd 200 with a DeploymentResponse
body (populate and return the deployment via the
deploymentService.UndeployDeploymentByHandle result) or, if you choose to keep
204, replace c.JSON(http.StatusNoContent, nil) with
c.Status(http.StatusNoContent) and ensure no body is sent. Also add explicit
error handling for a "deployment not currently active" error from
UndeployDeploymentByHandle (e.g., constants.ErrDeploymentNotActive or the
service-specific error) and map it to a 400 or 409 response instead of 500; keep
the existing mappings for constants.ErrAPINotFound and
constants.ErrDeploymentNotFound.
In `@platform-api/src/internal/repository/api.go`:
- Around line 1108-1123: The GetDeploymentContent function currently returns a
generic error when the deployment row is missing; change it to return the
package-level sentinel ErrDeploymentNotFound instead of errors.New("deployment
not found") so callers can map to 404s; locate APIRepo.GetDeploymentContent (and
the similar methods around GetDeploymentLogs/GetDeploymentMeta in the same file)
and replace the sql.ErrNoRows branch to return ErrDeploymentNotFound, keeping
other error returns unchanged and ensuring the sentinel is imported/defined in
the package.
- Around line 1327-1332: The LEFT JOIN to api_deployments (alias ad) can match
non-active or older deployments causing duplicates and incorrect is_deployed
flags; change the join to only include active deployments by adding a filter
ad.status = 'DEPLOYED' (and to avoid multiple rows per gateway select the latest
deployment—e.g., join against a subquery that for each gateway_uuid and api_uuid
returns the deployment with max(created_at) or max(deployment_id) before
joining). Update the JOIN condition on ad in the query that references g, aa, ad
(use the existing ON g.uuid = ad.gateway_uuid AND ad.api_uuid = ? plus the
status filter or replace ad with a deduplicated subquery) so is_deployed and
deployed_at reflect the latest deployed artifact only.
- Around line 964-989: In CreateDeployment, json.Marshal(deployment.Metadata)
currently ignores errors and can silently drop invalid metadata; capture the
error when marshaling deployment.Metadata (the code handling metadataJSON inside
CreateDeployment) and return or wrap that error instead of proceeding, ensuring
the INSERT does not run with missing/invalid metadata; update the logic that
sets metadataJSON (and any callers/returns) to handle the marshal error
appropriately and propagate it back to the caller.
In `@platform-api/src/internal/service/deployment.go`:
- Around line 239-244: Replace ad-hoc string errors for missing deployments with
the sentinel error constants.ErrDeploymentNotFound so callers using
errors.Is(...) will map to 404s: locate the branches that check the deployment
variable for nil (e.g., the block that currently returns errors.New("deployment
not found")) and change the return to constants.ErrDeploymentNotFound; do the
same for every other "not found" branch in this file (search for
errors.New("deployment not found") or similar) so all not-found returns
consistently use the ErrDeploymentNotFound sentinel.
- Around line 96-169: The deployment limit check in DeployAPI runs before
undeploying the current active deployment and only deletes one undeployed
artifact, which can exceed s.cfg.Deployments.MaxPerAPIGateway; also failures
from s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, ...,
"UNDEPLOYED", ...) are ignored—make them fatal. Fix by first attempting to
undeploy any existing active deployment via GetActiveDeploymentByGateway and
calling UpdateDeploymentStatus, returning an error if that call fails; then call
CountDeploymentsByAPIAndGateway and, while apiDeploymentCount >=
s.cfg.Deployments.MaxPerAPIGateway, fetch and delete the oldest undeployed
deployment using GetOldestUndeployedDeploymentByGateway and DeleteDeployment
(loop until under the limit), returning errors on any repo failures; apply the
same changes in RedeployDeployment.
In `@platform-api/src/resources/openapi.yaml`:
- Around line 946-973: The OpenAPI operation UndeployDeployment currently
declares a 200 response with schema DeploymentResponse but the implementation
returns 204 No Content; update the spec to match the implementation by replacing
the '200' response under operationId UndeployDeployment with a '204' response
that has description like "Deployment undeployed successfully" and no response
body (remove or stop referencing components/schemas/DeploymentResponse), or
alternatively change the handler to return 200 with the DeploymentResponse
payload—pick one approach and make the operationId UndeployDeployment, its
responses block, and any references to components/schemas/DeploymentResponse
consistent.
- Around line 910-945: The OpenAPI spec defines a required requestBody using the
RedeployRequest schema for the RedeployDeployment operation, but the
handler/service function redeploy currently expects no body; fix by either
making the requestBody optional in the spec (remove required: true or delete the
requestBody block for /apis/{apiId}/deployments/{deploymentId}/redeploy) or by
updating the redeploy implementation to accept and validate RedeployRequest
(consume gatewayId/metadata) and forward it to the redeploy service; ensure the
handler name (RedeployDeployment) and the service method redeploy accept the
same payload shape and update the components/schemas/RedeployRequest usage
accordingly, plus add input validation and tests for both paths.
- Around line 975-996: The OpenAPI response for operationId GetDeploymentContent
(path /apis/{apiId}/deployments/{deploymentId}/content) is incorrect: the
implementation returns raw YAML bytes but the spec declares application/json
with an object schema; update the spec to match the implementation by changing
the 200 response content type to application/x-yaml (or
application/octet-stream) and set the schema to a binary/string payload (e.g.,
type: string, format: binary or type: string with description "raw YAML bytes"),
so clients correctly interpret the raw YAML bytes instead of expecting JSON.
🧹 Nitpick comments (4)
platform-api/src/internal/service/gateway_events.go (1)
138-209: Extract shared broadcast helper to prevent driftBroadcastUndeploymentEvent duplicates the deployment broadcast flow (marshal, size check, connection loop, stats/logging). Consider a shared helper that takes event type/payload so future changes stay consistent across both paths.
platform-api/src/internal/dto/api.go (1)
165-188: Validate DeployAPIRequest.Base values
Baseis free-form but only"current"or a deployment ID are valid. Consider enforcing this via constants/validation to fail fast on bad requests and reduce downstream ambiguity.platform-api/src/internal/service/gateway_internal.go (1)
286-289: Avoid returning legacy deploymentId=0With deployment IDs now strings, returning
DeploymentId: 0can mislead clients. Consider deprecating this field or updating the response type to expose the new deployment ID.platform-api/src/internal/repository/api.go (1)
996-1065: Avoid fetching content blobs in list queries.
GetDeploymentsByAPIUUIDselectscontentfor every row, but the list response doesn’t expose it. This can be a large, unnecessary payload. Consider excludingcontent(or making it opt-in).♻️ Example change (apply to all query branches)
-SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, content, metadata, created_at +SELECT deployment_id, api_uuid, organization_uuid, gateway_uuid, status, base_deployment_id, metadata, created_at ... -err := rows.Scan(&deployment.DeploymentID, &deployment.ApiID, &deployment.OrganizationID, - &deployment.GatewayID, &deployment.Status, &baseDeploymentID, &deployment.Content, &metadataJSON, &deployment.CreatedAt) +err := rows.Scan(&deployment.DeploymentID, &deployment.ApiID, &deployment.OrganizationID, + &deployment.GatewayID, &deployment.Status, &baseDeploymentID, &metadataJSON, &deployment.CreatedAt)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
platform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/dto/api.goplatform-api/src/internal/handler/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/model/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/api.goplatform-api/src/internal/service/deployment.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/utils/api.goplatform-api/src/resources/openapi.yaml
💤 Files with no reviewable changes (3)
- platform-api/src/internal/handler/api.go
- platform-api/src/internal/utils/api.go
- platform-api/src/internal/service/api.go
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/config/config.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/server/server.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/repository/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/resources/openapi.yamlplatform-api/src/internal/service/deployment.go
📚 Learning: 2025-12-06T11:57:08.456Z
Learnt from: BLasan
Repo: wso2/api-platform PR: 140
File: gateway/gateway-controller/pkg/utils/api_deployment.go:88-105
Timestamp: 2025-12-06T11:57:08.456Z
Learning: In the gateway-controller Go codebase (file: gateway/gateway-controller/pkg/utils/api_deployment.go), only the async/websub API kind is currently supported. async/websocket and async/sse API kinds are not yet implemented.
Applied to files:
platform-api/src/internal/constants/error.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/server/server.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-15T11:14:32.797Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 686
File: platform-api/src/internal/service/api.go:920-922
Timestamp: 2026-01-15T11:14:32.797Z
Learning: In the platform-api service (file: platform-api/src/internal/service/api.go), API handles (the ID field) are immutable identifiers used as references and should not be updated after API creation. They serve as stable, user-facing identifiers for the API lifecycle.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-09T05:58:05.318Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/memory.go:484-503
Timestamp: 2026-01-09T05:58:05.318Z
Learning: In gateway/gateway-controller/pkg/storage/memory.go, during API key rotation via the StoreAPIKey method, the API key ID is never modified. When an existing API key is found by name (existingKeyID != ""), the incoming apiKey.ID is guaranteed to be the same as existingKeyID, so no ID mismatch can occur during updates.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_internal.go
📚 Learning: 2026-01-09T05:57:36.635Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/apikey_store.go:219-222
Timestamp: 2026-01-09T05:57:36.635Z
Learning: In gateway/gateway-controller/pkg/storage/apikey_store.go, during API key rotation, the API key ID and name remain constant—only the API key value changes. Therefore, when addToAPIMapping finds an existing key by name, the existingKeyID will always equal the incoming apiKey.ID.
Applied to files:
platform-api/src/internal/handler/gateway_internal.go
📚 Learning: 2025-12-18T08:33:51.340Z
Learnt from: O-sura
Repo: wso2/api-platform PR: 485
File: gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go:21-90
Timestamp: 2025-12-18T08:33:51.340Z
Learning: In gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go, the JSON tag casing differences between DefaultFaultEvent (correlationID, applicationID, organizationID, environmentID, regionID with uppercase 'ID') and DefaultResponseEvent (correlationId, applicationId, organizationId, environmentId, regionId with lowercase 'Id') are intentional to maintain consistency with respective event type conventions and config file property standards.
Applied to files:
platform-api/src/internal/model/gateway_event.go
🧬 Code graph analysis (8)
platform-api/src/internal/handler/gateway_internal.go (2)
platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/service/gateway_events.go (2)
platform-api/src/internal/model/gateway_event.go (1)
APIUndeploymentEvent(66-75)platform-api/src/internal/dto/gateway_event.go (1)
GatewayEventDTO(21-33)
platform-api/src/internal/server/server.go (2)
platform-api/src/internal/service/deployment.go (1)
NewDeploymentService(48-66)platform-api/src/internal/handler/deployment.go (1)
NewDeploymentHandler(38-42)
platform-api/src/internal/repository/api.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
platform-api/src/internal/dto/api.go (1)
platform-api/src/internal/dto/common.go (1)
Pagination(23-27)
platform-api/src/internal/handler/deployment.go (4)
platform-api/src/internal/middleware/auth.go (1)
GetOrganizationFromContext(169-176)platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/dto/api.go (1)
DeployAPIRequest(166-170)platform-api/src/internal/constants/error.go (3)
ErrAPINotFound(47-47)ErrGatewayNotFound(62-62)ErrDeploymentNotFound(68-68)
platform-api/src/internal/repository/interfaces.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
platform-api/src/internal/service/gateway_internal.go (1)
platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
🔇 Additional comments (24)
platform-api/src/internal/constants/error.go (1)
67-70: LGTM — deployment-specific errors are clearly defined.
Names/messages fit the existing error taxonomy and support new deployment flows.platform-api/src/config/config.go (2)
48-49: Deployment configuration is wired into the server config.
111-114: Default per-API gateway limit looks reasonable.platform-api/src/internal/model/gateway_event.go (1)
48-62: Deployment event payload aligns with the new deployment model.platform-api/src/internal/handler/gateway_internal.go (2)
20-32: Import update looks fine.
124-139: Active-deployment lookup and 404 mapping are appropriate.platform-api/src/internal/server/server.go (1)
104-105: Deployment service/handler wiring and route registration look correct.Also applies to: 115-116, 146-146
platform-api/src/internal/service/gateway_internal.go (1)
98-116: LGTM: active deployment lookupReturning
ErrDeploymentNotActiveon nil and using stored content keeps gateway retrieval clear and consistent.platform-api/src/internal/model/api.go (1)
242-252: Metadata persistence is already handled correctlyThe repository layer in
platform-api/src/internal/repository/api.goexplicitly marshalsMetadatato JSON string before insert and unmarshals it back to the map on retrieval. No runtime scan/insert failures will occur since JSON handling is implemented in the persistence layer, not relying ondatabase/sqlautomatic scanning.Likely an incorrect or invalid review comment.
platform-api/src/internal/handler/deployment.go (5)
34-42: Constructor wiring is clean.
Straightforward dependency injection.
44-98: Deploy handler validation and error mapping look solid.
277-312: List handler looks good.
357-368: Route registration is consistent.
100-142: Align redeploy handler with the published contract.
OpenAPI defines a requiredRedeployRequest(gatewayId/metadata), but this handler ignores the body and the service signature doesn't accept it; clients will send required data that gets dropped. Either remove/make the requestBody optional in the spec or bind and plumb it through. Also map the “deployment is already active” state to 400/409 instead of returning 500.⛔ Skipped due to learnings
Learnt from: thivindu Repo: wso2/api-platform PR: 110 File: platform-api/src/internal/service/api.go:432-476 Timestamp: 2025-11-08T13:06:22.133Z Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.platform-api/src/internal/repository/interfaces.go (1)
57-67: Deployment repository surface looks coherent.platform-api/src/internal/repository/api.go (4)
64-75: Column list updates are consistent across CRUD queries.Also applies to: 123-135, 179-200, 229-265, 365-374
1070-1106: GetDeploymentByID mapping and null handling look good.
1171-1248: Active/oldest deployment lookups are straightforward.
1408-1422: Deployment count helper looks good.platform-api/src/resources/openapi.yaml (3)
792-909: Deployment endpoints for create/list/get/delete are well documented.
3675-3693: Deployment request/response schemas look consistent.Also applies to: 3708-3768
3971-4010: New deployment parameters and tag are clear.platform-api/src/internal/service/deployment.go (2)
36-66: Service wiring looks good.
360-400: List/association helpers and handle-based variants look good.Also applies to: 454-480, 482-583
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| -- API Deployments table (immutable deployment artifacts) | ||
| CREATE TABLE IF NOT EXISTS api_deployments ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| deployment_id VARCHAR(40) PRIMARY KEY, | ||
| api_uuid VARCHAR(40) NOT NULL, | ||
| organization_uuid VARCHAR(40) NOT NULL, | ||
| gateway_uuid VARCHAR(40) NOT NULL, | ||
| status VARCHAR(20) NOT NULL DEFAULT 'DEPLOYED', | ||
| base_deployment_id VARCHAR(40), -- Reference to the deployment used as base, NULL if based on "current" | ||
| content BLOB NOT NULL, -- Immutable deployment artifact | ||
| metadata TEXT, -- JSON object for flexible key-value metadata | ||
| created_at DATETIME DEFAULT CURRENT_TIMESTAMP, | ||
| FOREIGN KEY (api_uuid) REFERENCES apis(uuid) ON DELETE CASCADE, | ||
| FOREIGN KEY (organization_uuid) REFERENCES organizations(uuid) ON DELETE CASCADE, | ||
| FOREIGN KEY (gateway_uuid) REFERENCES gateways(uuid) ON DELETE CASCADE, | ||
| UNIQUE(api_uuid, gateway_uuid) | ||
| UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid) | ||
| CHECK (status IN ('DEPLOYED', 'UNDEPLOYED')) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n platform-api/src/internal/database/schema.sql | sed -n '220,236p'Repository: wso2/api-platform
Length of output: 1131
🌐 Web query:
SQLite CREATE TABLE syntax multiple constraints comma requirement
💡 Result:
Yes — column definitions and table-constraint clauses in a CREATE TABLE statement form a comma-separated list, so you must separate multiple constraints (and columns) with commas. See the SQLite CREATE TABLE documentation which describes column definitions optionally followed by a list of table constraints (comma-separated). [1]
Example (valid SQLite):
CREATE TABLE t (
a INTEGER NOT NULL,
b TEXT,
PRIMARY KEY(a,b),
CHECK(length(b)>0)
);
Reference: SQLite CREATE TABLE documentation. [1]
Add missing comma between UNIQUE and CHECK constraints
Line 234–235: Missing comma after the UNIQUE constraint. SQLite requires comma-separated table constraints in CREATE TABLE statements.
Fix
- UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid)
- CHECK (status IN ('DEPLOYED', 'UNDEPLOYED'))
+ UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid),
+ CHECK (status IN ('DEPLOYED', 'UNDEPLOYED'))🤖 Prompt for AI Agents
In `@platform-api/src/internal/database/schema.sql` around lines 220 - 236, The
CREATE TABLE for api_deployments has a missing comma between the table
constraints UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid) and
CHECK (status IN (...)), which causes a SQL syntax error; add a comma after the
UNIQUE(...) constraint in the api_deployments table definition so the UNIQUE
constraint and the CHECK constraint are properly separated and the table
(functionally api_deployments) can be created.
| // UndeployDeployment handles POST /api/v1/apis/:apiId/deployments/:deploymentId/undeploy | ||
| // Undeploys an active deployment by changing its status to UNDEPLOYED | ||
| func (h *DeploymentHandler) UndeployDeployment(c *gin.Context) { | ||
| orgId, exists := middleware.GetOrganizationFromContext(c) | ||
| if !exists { | ||
| c.JSON(http.StatusUnauthorized, utils.NewErrorResponse(401, "Unauthorized", | ||
| "Organization claim not found in token")) | ||
| return | ||
| } | ||
|
|
||
| apiId := c.Param("apiId") | ||
| deploymentId := c.Param("deploymentId") | ||
|
|
||
| if apiId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "API ID is required")) | ||
| return | ||
| } | ||
| if deploymentId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "Deployment ID is required")) | ||
| return | ||
| } | ||
|
|
||
| err := h.deploymentService.UndeployDeploymentByHandle(apiId, deploymentId, orgId) | ||
| if err != nil { | ||
| if errors.Is(err, constants.ErrAPINotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "API not found")) | ||
| return | ||
| } | ||
| if errors.Is(err, constants.ErrDeploymentNotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "Deployment not found")) | ||
| return | ||
| } | ||
| log.Printf("[ERROR] Failed to undeploy: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | ||
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", err.Error())) | ||
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusNoContent, nil) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status/contract mismatch for undeploy.
The handler returns 204 (and sends JSON null) while the spec declares 200 + DeploymentResponse. Align implementation/spec; if you keep 204, use c.Status(204) and update the spec. Also map “deployment is not currently active” to 400/409 instead of 500.
🛠️ If keeping 204, avoid a JSON body
- c.JSON(http.StatusNoContent, nil)
+ c.Status(http.StatusNoContent)🤖 Prompt for AI Agents
In `@platform-api/src/internal/handler/deployment.go` around lines 144 - 186,
Handler UndeployDeployment currently sends a 204 with a JSON null
(c.JSON(http.StatusNoContent, nil)) which mismatches the API contract; update
the handler to either return the spec'd 200 with a DeploymentResponse body
(populate and return the deployment via the
deploymentService.UndeployDeploymentByHandle result) or, if you choose to keep
204, replace c.JSON(http.StatusNoContent, nil) with
c.Status(http.StatusNoContent) and ensure no body is sent. Also add explicit
error handling for a "deployment not currently active" error from
UndeployDeploymentByHandle (e.g., constants.ErrDeploymentNotActive or the
service-specific error) and map it to a 400 or 409 response instead of 500; keep
the existing mappings for constants.ErrAPINotFound and
constants.ErrDeploymentNotFound.
| // DeleteDeployment handles DELETE /api/v1/apis/:apiId/deployments/:deploymentId | ||
| // Permanently deletes an undeployed deployment artifact | ||
| func (h *DeploymentHandler) DeleteDeployment(c *gin.Context) { | ||
| orgId, exists := middleware.GetOrganizationFromContext(c) | ||
| if !exists { | ||
| c.JSON(http.StatusUnauthorized, utils.NewErrorResponse(401, "Unauthorized", | ||
| "Organization claim not found in token")) | ||
| return | ||
| } | ||
|
|
||
| apiId := c.Param("apiId") | ||
| deploymentId := c.Param("deploymentId") | ||
|
|
||
| if apiId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "API ID is required")) | ||
| return | ||
| } | ||
| if deploymentId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "Deployment ID is required")) | ||
| return | ||
| } | ||
|
|
||
| err := h.deploymentService.DeleteDeploymentByHandle(apiId, deploymentId, orgId) | ||
| if err != nil { | ||
| if errors.Is(err, constants.ErrAPINotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "API not found")) | ||
| return | ||
| } | ||
| if errors.Is(err, constants.ErrDeploymentNotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "Deployment not found")) | ||
| return | ||
| } | ||
| log.Printf("[ERROR] Failed to delete deployment: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | ||
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", err.Error())) | ||
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusNoContent, nil) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return a conflict instead of 500 for active deployments.
Service errors like “cannot delete an active deployment” are currently surfaced as 500, but the spec documents 409. Add explicit error mapping (and consider c.Status(204) if you intend no body).
🤖 Prompt for AI Agents
In `@platform-api/src/internal/handler/deployment.go` around lines 188 - 230, The
DeleteDeployment handler currently maps service errors to 500; update error
handling after calling deploymentService.DeleteDeploymentByHandle(apiId,
deploymentId, orgId) to map the service error that indicates an active
deployment (e.g., constants.ErrDeploymentActive or the specific error returned
by your service) to HTTP 409 with an appropriate utils.NewErrorResponse(409,
"Conflict", "<human message>"), leaving other errors as-is; also return no body
for success by using c.Status(http.StatusNoContent) (or
c.JSON(http.StatusNoContent, nil) consistently) instead of sending a body with
204.
| deployment, err := h.deploymentService.GetDeploymentByHandle(apiId, deploymentId, orgId) | ||
| if err != nil { | ||
| if errors.Is(err, constants.ErrAPINotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "API not found")) | ||
| return | ||
| } | ||
| if errors.Is(err, constants.ErrDeploymentNotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "Deployment not found")) | ||
| return | ||
| } | ||
| log.Printf("[ERROR] Failed to get deployment: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | ||
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | ||
| "Deployment not found")) | ||
| return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t mask internal errors as “not found.”
Use err.Error() or “Failed to retrieve deployment” to avoid misleading 500s.
💡 Suggested fix
- c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
- "Deployment not found"))
+ c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
+ err.Error()))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deployment, err := h.deploymentService.GetDeploymentByHandle(apiId, deploymentId, orgId) | |
| if err != nil { | |
| if errors.Is(err, constants.ErrAPINotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "API not found")) | |
| return | |
| } | |
| if errors.Is(err, constants.ErrDeploymentNotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "Deployment not found")) | |
| return | |
| } | |
| log.Printf("[ERROR] Failed to get deployment: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | |
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | |
| "Deployment not found")) | |
| return | |
| deployment, err := h.deploymentService.GetDeploymentByHandle(apiId, deploymentId, orgId) | |
| if err != nil { | |
| if errors.Is(err, constants.ErrAPINotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "API not found")) | |
| return | |
| } | |
| if errors.Is(err, constants.ErrDeploymentNotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "Deployment not found")) | |
| return | |
| } | |
| log.Printf("[ERROR] Failed to get deployment: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | |
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | |
| err.Error())) | |
| return |
🤖 Prompt for AI Agents
In `@platform-api/src/internal/handler/deployment.go` around lines 256 - 271, The
500 response currently returns "Deployment not found" which masks internal
errors; in the error branch after calling
h.deploymentService.GetDeploymentByHandle (and after checking
constants.ErrAPINotFound and constants.ErrDeploymentNotFound) change the c.JSON
internal server error body to include the actual error details (e.g.,
err.Error() or a message like "Failed to retrieve deployment") instead of
"Deployment not found" so the response accurately reflects the internal failure
while keeping the existing log.Printf call.
| // GetDeploymentContent handles GET /api/v1/apis/:apiId/deployments/:deploymentId/content | ||
| // Retrieves the immutable content blob for a deployment | ||
| func (h *DeploymentHandler) GetDeploymentContent(c *gin.Context) { | ||
| orgId, exists := middleware.GetOrganizationFromContext(c) | ||
| if !exists { | ||
| c.JSON(http.StatusUnauthorized, utils.NewErrorResponse(401, "Unauthorized", | ||
| "Organization claim not found in token")) | ||
| return | ||
| } | ||
|
|
||
| apiId := c.Param("apiId") | ||
| deploymentId := c.Param("deploymentId") | ||
|
|
||
| if apiId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "API ID is required")) | ||
| return | ||
| } | ||
| if deploymentId == "" { | ||
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | ||
| "Deployment ID is required")) | ||
| return | ||
| } | ||
|
|
||
| content, err := h.deploymentService.GetDeploymentContentByHandle(apiId, deploymentId, orgId) | ||
| if err != nil { | ||
| if errors.Is(err, constants.ErrAPINotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "API not found")) | ||
| return | ||
| } | ||
| if errors.Is(err, constants.ErrDeploymentNotFound) { | ||
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | ||
| "Deployment not found")) | ||
| return | ||
| } | ||
| log.Printf("[ERROR] Failed to get deployment content: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | ||
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | ||
| "Failed to retrieve deployment content")) | ||
| } | ||
| c.Data(http.StatusOK, "application/json", content) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return after error and serve the correct content type.
After sending error JSON, the handler falls through and writes a second response. Also artifacts are stored as YAML (GenerateAPIDeploymentYAML), so application/json misleads clients. Add a return and align media type/spec (or convert to JSON).
🔧 Suggested fix
- c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
- "Failed to retrieve deployment content"))
- }
- c.Data(http.StatusOK, "application/json", content)
+ c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
+ "Failed to retrieve deployment content"))
+ return
+ }
+ c.Data(http.StatusOK, "application/x-yaml", content)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // GetDeploymentContent handles GET /api/v1/apis/:apiId/deployments/:deploymentId/content | |
| // Retrieves the immutable content blob for a deployment | |
| func (h *DeploymentHandler) GetDeploymentContent(c *gin.Context) { | |
| orgId, exists := middleware.GetOrganizationFromContext(c) | |
| if !exists { | |
| c.JSON(http.StatusUnauthorized, utils.NewErrorResponse(401, "Unauthorized", | |
| "Organization claim not found in token")) | |
| return | |
| } | |
| apiId := c.Param("apiId") | |
| deploymentId := c.Param("deploymentId") | |
| if apiId == "" { | |
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | |
| "API ID is required")) | |
| return | |
| } | |
| if deploymentId == "" { | |
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | |
| "Deployment ID is required")) | |
| return | |
| } | |
| content, err := h.deploymentService.GetDeploymentContentByHandle(apiId, deploymentId, orgId) | |
| if err != nil { | |
| if errors.Is(err, constants.ErrAPINotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "API not found")) | |
| return | |
| } | |
| if errors.Is(err, constants.ErrDeploymentNotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "Deployment not found")) | |
| return | |
| } | |
| log.Printf("[ERROR] Failed to get deployment content: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | |
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | |
| "Failed to retrieve deployment content")) | |
| } | |
| c.Data(http.StatusOK, "application/json", content) | |
| } | |
| // GetDeploymentContent handles GET /api/v1/apis/:apiId/deployments/:deploymentId/content | |
| // Retrieves the immutable content blob for a deployment | |
| func (h *DeploymentHandler) GetDeploymentContent(c *gin.Context) { | |
| orgId, exists := middleware.GetOrganizationFromContext(c) | |
| if !exists { | |
| c.JSON(http.StatusUnauthorized, utils.NewErrorResponse(401, "Unauthorized", | |
| "Organization claim not found in token")) | |
| return | |
| } | |
| apiId := c.Param("apiId") | |
| deploymentId := c.Param("deploymentId") | |
| if apiId == "" { | |
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | |
| "API ID is required")) | |
| return | |
| } | |
| if deploymentId == "" { | |
| c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", | |
| "Deployment ID is required")) | |
| return | |
| } | |
| content, err := h.deploymentService.GetDeploymentContentByHandle(apiId, deploymentId, orgId) | |
| if err != nil { | |
| if errors.Is(err, constants.ErrAPINotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "API not found")) | |
| return | |
| } | |
| if errors.Is(err, constants.ErrDeploymentNotFound) { | |
| c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found", | |
| "Deployment not found")) | |
| return | |
| } | |
| log.Printf("[ERROR] Failed to get deployment content: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | |
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | |
| "Failed to retrieve deployment content")) | |
| return | |
| } | |
| c.Data(http.StatusOK, "application/x-yaml", content) | |
| } |
🤖 Prompt for AI Agents
In `@platform-api/src/internal/handler/deployment.go` around lines 314 - 355, In
GetDeploymentContent, the handler currently falls through after sending error
JSON (causing a second response) and sends the wrong media type; add a return
immediately after each c.JSON error response in the error branch of
h.deploymentService.GetDeploymentContentByHandle so the function exits on error,
and change the final c.Data content type from "application/json" to the correct
YAML media type (e.g., "application/x-yaml" or "text/yaml") to match
GenerateAPIDeploymentYAML-produced artifacts.
| // Check deployment limits | ||
| apiDeploymentCount, err := s.apiRepo.CountDeploymentsByAPIAndGateway(apiUUID, req.GatewayID, orgUUID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to check deployment count: %w", err) | ||
| } | ||
| if apiDeploymentCount >= s.cfg.Deployments.MaxPerAPIGateway { | ||
| // Delete oldest deployment in UNDEPLOYED state to make room | ||
| oldestDeployment, err := s.apiRepo.GetOldestUndeployedDeploymentByGateway(apiUUID, req.GatewayID, orgUUID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get oldest undeployed deployment: %w", err) | ||
| } | ||
| if oldestDeployment != nil { | ||
| if err := s.apiRepo.DeleteDeployment(oldestDeployment.DeploymentID, apiUUID, orgUUID); err != nil { | ||
| return nil, fmt.Errorf("failed to delete oldest undeployed deployment: %w", err) | ||
| } | ||
| log.Printf("[INFO] Deleted oldest undeployed deployment %s to make room for new deployment", oldestDeployment.DeploymentID) | ||
| } | ||
| } | ||
| // Validate API has backend services attached | ||
| backendServices, err := s.backendServiceRepo.GetBackendServicesByAPIID(apiUUID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get backend services: %w", err) | ||
| } | ||
| if len(backendServices) == 0 { | ||
| return nil, errors.New("API must have at least one backend service attached before deployment") | ||
| } | ||
|
|
||
| var baseDeploymentID *string | ||
| var apiContent *dto.API | ||
| var contentBytes []byte | ||
|
|
||
| // Determine the source: "current" or existing deployment | ||
| if req.Base == "current" { | ||
| // Use current API state | ||
| apiContent = s.apiUtil.ModelToDTO(apiModel) | ||
|
|
||
| // Generate API deployment YAML for storage | ||
| apiYaml, err := s.apiUtil.GenerateAPIDeploymentYAML(apiContent) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to generate API deployment YAML: %w", err) | ||
| } | ||
|
|
||
| // Create immutable deployment artifact content (store as YAML bytes) | ||
| contentBytes = []byte(apiYaml) | ||
| } else { | ||
| // Use existing deployment as base | ||
| baseDeployment, err := s.apiRepo.GetDeploymentByID(req.Base, apiUUID, orgUUID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get base deployment: %w", err) | ||
| } | ||
| if baseDeployment == nil { | ||
| return nil, errors.New("base deployment not found") | ||
| } | ||
|
|
||
| // Deployment content is already stored as YAML, reuse it directly | ||
| contentBytes = baseDeployment.Content | ||
| baseDeploymentID = &req.Base | ||
| } | ||
|
|
||
| // Generate deployment ID | ||
| deploymentID := uuid.New().String() | ||
|
|
||
| // Check if there's an existing active deployment on this gateway | ||
| existingDeployment, err := s.apiRepo.GetActiveDeploymentByGateway(apiUUID, req.GatewayID, orgUUID) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to check existing deployments: %w", err) | ||
| } | ||
|
|
||
| // If exists, undeploy it | ||
| if existingDeployment != nil { | ||
| if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil { | ||
| log.Printf("[WARN] Failed to undeploy existing deployment %s: %v", existingDeployment.DeploymentID, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce deployment limits and fail fast on undeploy failures.
The limit check runs before undeploying the current active deployment and deletes at most one undeployed artifact; this can exceed MaxPerAPIGateway (e.g., when max=1). Also, both DeployAPI and RedeployDeployment ignore failures when marking the existing deployment UNDEPLOYED, which can leave multiple active deployments. Treat those failures as fatal and re-check/trim counts after the undeploy step.
💡 Suggested fix
- if apiDeploymentCount >= s.cfg.Deployments.MaxPerAPIGateway {
- // Delete oldest deployment in UNDEPLOYED state to make room
- oldestDeployment, err := s.apiRepo.GetOldestUndeployedDeploymentByGateway(apiUUID, req.GatewayID, orgUUID)
- if err != nil {
- return nil, fmt.Errorf("failed to get oldest undeployed deployment: %w", err)
- }
- if oldestDeployment != nil {
- if err := s.apiRepo.DeleteDeployment(oldestDeployment.DeploymentID, apiUUID, orgUUID); err != nil {
- return nil, fmt.Errorf("failed to delete oldest undeployed deployment: %w", err)
- }
- log.Printf("[INFO] Deleted oldest undeployed deployment %s to make room for new deployment", oldestDeployment.DeploymentID)
- }
- }
+ cleanupOldest := func() error {
+ for apiDeploymentCount >= s.cfg.Deployments.MaxPerAPIGateway {
+ oldestDeployment, err := s.apiRepo.GetOldestUndeployedDeploymentByGateway(apiUUID, req.GatewayID, orgUUID)
+ if err != nil {
+ return fmt.Errorf("failed to get oldest undeployed deployment: %w", err)
+ }
+ if oldestDeployment == nil {
+ return fmt.Errorf("deployment limit reached for api %s on gateway %s", apiUUID, req.GatewayID)
+ }
+ if err := s.apiRepo.DeleteDeployment(oldestDeployment.DeploymentID, apiUUID, orgUUID); err != nil {
+ return fmt.Errorf("failed to delete oldest undeployed deployment: %w", err)
+ }
+ apiDeploymentCount--
+ }
+ return nil
+ }
@@
- if existingDeployment != nil {
- if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil {
- log.Printf("[WARN] Failed to undeploy existing deployment %s: %v", existingDeployment.DeploymentID, err)
- }
- }
+ if existingDeployment != nil {
+ if err := s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, apiUUID, "UNDEPLOYED", orgUUID); err != nil {
+ return nil, fmt.Errorf("failed to undeploy existing deployment %s: %w", existingDeployment.DeploymentID, err)
+ }
+ }
+ if err := cleanupOldest(); err != nil {
+ return nil, err
+ }Also applies to: 246-256
🤖 Prompt for AI Agents
In `@platform-api/src/internal/service/deployment.go` around lines 96 - 169, The
deployment limit check in DeployAPI runs before undeploying the current active
deployment and only deletes one undeployed artifact, which can exceed
s.cfg.Deployments.MaxPerAPIGateway; also failures from
s.apiRepo.UpdateDeploymentStatus(existingDeployment.DeploymentID, ...,
"UNDEPLOYED", ...) are ignored—make them fatal. Fix by first attempting to
undeploy any existing active deployment via GetActiveDeploymentByGateway and
calling UpdateDeploymentStatus, returning an error if that call fails; then call
CountDeploymentsByAPIAndGateway and, while apiDeploymentCount >=
s.cfg.Deployments.MaxPerAPIGateway, fetch and delete the oldest undeployed
deployment using GetOldestUndeployedDeploymentByGateway and DeleteDeployment
(loop until under the limit), returning errors on any repo failures; apply the
same changes in RedeployDeployment.
| if deployment == nil { | ||
| return nil, errors.New("deployment not found") | ||
| } | ||
| if deployment.Status == "DEPLOYED" { | ||
| return nil, errors.New("deployment is already active") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return ErrDeploymentNotFound for 404 mapping.
Handlers check errors.Is(..., constants.ErrDeploymentNotFound); these string errors will currently surface as 500s. Use the sentinel error in all “not found” branches.
💡 Suggested fix (apply to all not-found branches)
- if deployment == nil {
- return nil, errors.New("deployment not found")
- }
+ if deployment == nil {
+ return nil, constants.ErrDeploymentNotFound
+ }Also applies to: 305-309, 345-349, 418-420, 441-443
🤖 Prompt for AI Agents
In `@platform-api/src/internal/service/deployment.go` around lines 239 - 244,
Replace ad-hoc string errors for missing deployments with the sentinel error
constants.ErrDeploymentNotFound so callers using errors.Is(...) will map to
404s: locate the branches that check the deployment variable for nil (e.g., the
block that currently returns errors.New("deployment not found")) and change the
return to constants.ErrDeploymentNotFound; do the same for every other "not
found" branch in this file (search for errors.New("deployment not found") or
similar) so all not-found returns consistently use the ErrDeploymentNotFound
sentinel.
| /apis/{apiId}/deployments/{deploymentId}/redeploy: | ||
| post: | ||
| summary: Redeploy deployment | ||
| description: | | ||
| Redeploys an existing deployment artifact. If a previous deployment exists on the same gateway, | ||
| it will be automatically undeployed first. Access is validated against the organization | ||
| in the JWT token. | ||
| operationId: RedeployDeployment | ||
| tags: | ||
| - API Deployments | ||
| parameters: | ||
| - $ref: '#/components/parameters/apiID' | ||
| - $ref: '#/components/parameters/deploymentId' | ||
| requestBody: | ||
| description: Redeploy request details | ||
| required: true | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/RedeployRequest' | ||
| responses: | ||
| '200': | ||
| description: Deployment redeployed successfully | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/DeploymentResponse' | ||
| '400': | ||
| $ref: '#/components/responses/BadRequest' | ||
| '401': | ||
| $ref: '#/components/responses/Unauthorized' | ||
| '404': | ||
| $ref: '#/components/responses/NotFound' | ||
| '500': | ||
| $ref: '#/components/responses/InternalServerError' | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redeploy request body doesn’t match implementation.
The handler/service redeploy with no body, but the spec requires RedeployRequest (gatewayId/metadata). Either make the requestBody optional/remove it or update the implementation to consume it.
Also applies to: 3694-3707
🤖 Prompt for AI Agents
In `@platform-api/src/resources/openapi.yaml` around lines 910 - 945, The OpenAPI
spec defines a required requestBody using the RedeployRequest schema for the
RedeployDeployment operation, but the handler/service function redeploy
currently expects no body; fix by either making the requestBody optional in the
spec (remove required: true or delete the requestBody block for
/apis/{apiId}/deployments/{deploymentId}/redeploy) or by updating the redeploy
implementation to accept and validate RedeployRequest (consume
gatewayId/metadata) and forward it to the redeploy service; ensure the handler
name (RedeployDeployment) and the service method redeploy accept the same
payload shape and update the components/schemas/RedeployRequest usage
accordingly, plus add input validation and tests for both paths.
| /apis/{apiId}/deployments/{deploymentId}/undeploy: | ||
| post: | ||
| summary: Undeploy deployment | ||
| description: | | ||
| Undeploys an active deployment, stopping the API from being served. The deployment artifact | ||
| remains in the system with UNDEPLOYED status and can be redeployed later. Access is validated | ||
| against the organization in the JWT token. | ||
| operationId: UndeployDeployment | ||
| tags: | ||
| - API Deployments | ||
| parameters: | ||
| - $ref: '#/components/parameters/apiID' | ||
| - $ref: '#/components/parameters/deploymentId' | ||
| responses: | ||
| '200': | ||
| description: Deployment undeployed successfully | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/DeploymentResponse' | ||
| '400': | ||
| $ref: '#/components/responses/BadRequest' | ||
| '401': | ||
| $ref: '#/components/responses/Unauthorized' | ||
| '404': | ||
| $ref: '#/components/responses/NotFound' | ||
| '500': | ||
| $ref: '#/components/responses/InternalServerError' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undeploy response mismatch.
Spec declares 200 + DeploymentResponse, but the handler returns 204. Align the contract with the implementation.
🤖 Prompt for AI Agents
In `@platform-api/src/resources/openapi.yaml` around lines 946 - 973, The OpenAPI
operation UndeployDeployment currently declares a 200 response with schema
DeploymentResponse but the implementation returns 204 No Content; update the
spec to match the implementation by replacing the '200' response under
operationId UndeployDeployment with a '204' response that has description like
"Deployment undeployed successfully" and no response body (remove or stop
referencing components/schemas/DeploymentResponse), or alternatively change the
handler to return 200 with the DeploymentResponse payload—pick one approach and
make the operationId UndeployDeployment, its responses block, and any references
to components/schemas/DeploymentResponse consistent.
| /apis/{apiId}/deployments/{deploymentId}/content: | ||
| get: | ||
| summary: Get deployment artifact content | ||
| description: | | ||
| Retrieves the full content of a deployment artifact. The artifact contains the complete | ||
| API configuration and policies as they were at deployment time. Access is validated | ||
| against the organization in the JWT token. | ||
| operationId: GetDeploymentContent | ||
| tags: | ||
| - API Deployments | ||
| parameters: | ||
| - $ref: '#/components/parameters/apiID' | ||
| - $ref: '#/components/parameters/deploymentId' | ||
| responses: | ||
| '200': | ||
| description: Deployment content retrieved successfully | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| description: Raw deployment artifact content | ||
| '401': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deployment content media type/schema mismatch.
The implementation serves raw YAML bytes; the spec says application/json with an object schema. Update the spec (or convert to JSON) to avoid client parsing failures.
🤖 Prompt for AI Agents
In `@platform-api/src/resources/openapi.yaml` around lines 975 - 996, The OpenAPI
response for operationId GetDeploymentContent (path
/apis/{apiId}/deployments/{deploymentId}/content) is incorrect: the
implementation returns raw YAML bytes but the spec declares application/json
with an object schema; update the spec to match the implementation by changing
the 200 response content type to application/x-yaml (or
application/octet-stream) and set the schema to a binary/string payload (e.g.,
type: string, format: binary or type: string with description "raw YAML bytes"),
so clients correctly interpret the raw YAML bytes instead of expecting JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platform-api/src/internal/service/gateway_internal.go (1)
273-281: Add Status and Content fields to deployment record before insert.The
APIDeploymentcreation at lines 273–281 omitsStatusandContent, both of which areNOT NULLin the schema with no repository-side defaults. Thecontentcolumn has no default value, so the INSERT will fail. Even if it succeeded,GetActiveDeploymentByGatewayexpects to readdeployment.Contentand would return empty data.Compare with the proper pattern in
deployment.go(lines 187–195), which setsStatus: "DEPLOYED"and populatesContentbefore callingCreateDeployment. Follow that same approach here:Suggested approach
// Create deployment record deployment := &model.APIDeployment{ + Status: "DEPLOYED", + Content: []byte(notification.Configuration), // or serialize as needed ApiID: apiUUID, GatewayID: gatewayID, OrganizationID: orgID, CreatedAt: now, }
🤖 Fix all issues with AI agents
In `@platform-api/src/internal/dto/api.go`:
- Around line 172-188: Update the DTOs to match the OpenAPI contract: add an
OrganizationID string field to DeploymentResponse (json:"organizationId"
yaml:"organizationId"), and change DeploymentListResponse to expose Deployments
[]*DeploymentResponse `json:"deployments" yaml:"deployments"` instead of
Count/List/Pagination (i.e., remove Count and Pagination and rename List to
Deployments). Also update any code that constructs or consumes
DeploymentResponse and DeploymentListResponse (handlers/serializers) to populate
OrganizationID and to produce the Deployments slice (and stop expecting
Count/Pagination).
In `@platform-api/src/internal/service/deployment.go`:
- Around line 201-204: The code currently swallows failures from
s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID) by logging a
warning; instead treat association failures as fatal: propagate the error (or
wrap it with context) and return it from the enclosing function so the
deployment is aborted/rolled back rather than reported as successful. Update the
surrounding control flow to return the error from the caller (or trigger the
existing rollback path) and include contextual information (apiUUID,
req.GatewayID, orgUUID) in the returned/wrapped error to aid debugging.
- Around line 575-583: GetDeploymentContentByHandle builds apiUUID via
getAPIUUIDByHandle but then calls GetDeploymentContent with the arguments
swapped; change the call in GetDeploymentContentByHandle to pass deploymentID,
apiUUID, orgUUID (instead of apiUUID, deploymentID, orgUUID) so
GetDeploymentContent receives (deploymentID, apiUUID, orgUUID) in the correct
order.
♻️ Duplicate comments (13)
platform-api/src/internal/database/schema.sql (1)
234-235: Add missing comma between UNIQUE and CHECK constraints.SQLite requires comma-separated table constraints. This will cause a syntax error when creating the table.
Fix
- UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid) - CHECK (status IN ('DEPLOYED', 'UNDEPLOYED')) + UNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid), + CHECK (status IN ('DEPLOYED', 'UNDEPLOYED'))platform-api/src/internal/handler/deployment.go (4)
168-185: Align undeploy response and error mapping with the contract.
Still returns 204 with a JSON body and doesn’t map “not active” to a 4xx/409.
212-229: Return conflict for active deployments and avoid body on 204.
Delete should map “active deployment” errors to 409 and usec.Status(204)if keeping No Content.
256-271: Don’t mask internal errors as “not found.”
The 500 path currently returns “Deployment not found,” which is misleading.
338-355: Return after errors and use the correct content type.
The handler falls through after error JSON and serves YAML as JSON.platform-api/src/resources/openapi.yaml (3)
910-930: Redeploy request body doesn’t match implementation.
Spec requiresRedeployRequest, but redeploy currently accepts no body. Make the requestBody optional/remove it or update the handler/service to consume it.
946-966: Undeploy response mismatch.
Spec declares 200 +DeploymentResponse, but the implementation returns 204 No Content. Align the contract.
975-995: Deployment content media type/schema mismatch.
Implementation returns raw YAML bytes but spec declares JSON object. Update toapplication/x-yaml(or binary) and a string/binary schema.platform-api/src/internal/repository/api.go (3)
984-987: Handle metadata marshal failures.
json.Marshalerrors are ignored, so invalid metadata is silently dropped. Return the error to avoid storing corrupted/empty metadata.💡 Suggested fix
var metadataJSON string if deployment.Metadata != nil { - metadataBytes, _ := json.Marshal(deployment.Metadata) - metadataJSON = string(metadataBytes) + metadataBytes, mErr := json.Marshal(deployment.Metadata) + if mErr != nil { + return fmt.Errorf("failed to marshal deployment metadata: %w", mErr) + } + metadataJSON = string(metadataBytes) }
1114-1120: UseErrDeploymentNotFoundfor not-found cases.
String errors prevent callers from mapping to 404. Return the sentinel consistently.Also applies to: 1138-1145, 1159-1166
1327-1332: Filter deployment join to active deployments.
Joining all deployments can duplicate rows and markis_deployedtrue even when only undeployed artifacts exist. Filter bystatus='DEPLOYED'(and ideally select latest).platform-api/src/internal/service/deployment.go (2)
96-113: Enforce deployment limits after undeploying and fail fast on undeploy errors.
Current flow checks limits before undeploy and logs undeploy failures; this can exceedMaxPerAPIGatewayor leave multiple active deployments. Make undeploy failures fatal and trim until under the limit after the undeploy step.Also applies to: 164-168, 252-256
239-241: UseErrDeploymentNotFoundfor missing deployments.
Handlers map this sentinel to 404; string errors will surface as 500s.Also applies to: 304-306, 345-347, 418-420, 441-443
🧹 Nitpick comments (2)
platform-api/src/internal/service/gateway_events.go (1)
137-209: Consider extracting common broadcast logic to reduce duplication.The
BroadcastUndeploymentEventmethod duplicates nearly all ofBroadcastDeploymentEvent(correlation ID generation, serialization, size validation, connection lookup, broadcast loop, stats tracking). Consider extracting a generic helper:func (s *GatewayEventsService) broadcastEvent(gatewayID string, eventType string, payload interface{}) errorThis would reduce maintenance burden and ensure consistent behavior across event types.
platform-api/src/internal/database/schema.sql (1)
220-236: Consider adding indexes for common deployment query patterns.The
api_deploymentstable lacks indexes for common queries. Based on the deployment service operations mentioned in the AI summary, consider adding:CREATE INDEX IF NOT EXISTS idx_api_deployments_api ON api_deployments(api_uuid); CREATE INDEX IF NOT EXISTS idx_api_deployments_gateway_status ON api_deployments(gateway_uuid, status); CREATE INDEX IF NOT EXISTS idx_api_deployments_org ON api_deployments(organization_uuid);This would optimize queries for fetching deployments by API, finding active deployments by gateway, and listing deployments by organization.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
platform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/dto/api.goplatform-api/src/internal/handler/api.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/model/api.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/api.goplatform-api/src/internal/service/deployment.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/utils/api.goplatform-api/src/resources/openapi.yaml
💤 Files with no reviewable changes (3)
- platform-api/src/internal/service/api.go
- platform-api/src/internal/utils/api.go
- platform-api/src/internal/handler/api.go
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
📚 Learning: 2025-11-08T13:06:22.133Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 110
File: platform-api/src/internal/service/api.go:432-476
Timestamp: 2025-11-08T13:06:22.133Z
Learning: In the platform-api Go codebase (file: platform-api/src/internal/service/api.go), the DeployAPIRevision method is designed to automatically create API-gateway associations during deployment if they don't already exist. There is no requirement that associations must pre-exist before deployment; the system checks for existing associations and creates them on-the-fly as needed.
Applied to files:
platform-api/src/internal/service/gateway_events.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/config/config.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/constants/error.goplatform-api/src/resources/openapi.yamlplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2025-12-06T11:57:08.456Z
Learnt from: BLasan
Repo: wso2/api-platform PR: 140
File: gateway/gateway-controller/pkg/utils/api_deployment.go:88-105
Timestamp: 2025-12-06T11:57:08.456Z
Learning: In the gateway-controller Go codebase (file: gateway/gateway-controller/pkg/utils/api_deployment.go), only the async/websub API kind is currently supported. async/websocket and async/sse API kinds are not yet implemented.
Applied to files:
platform-api/src/internal/service/gateway_events.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-15T11:14:32.797Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 686
File: platform-api/src/internal/service/api.go:920-922
Timestamp: 2026-01-15T11:14:32.797Z
Learning: In the platform-api service (file: platform-api/src/internal/service/api.go), API handles (the ID field) are immutable identifiers used as references and should not be updated after API creation. They serve as stable, user-facing identifiers for the API lifecycle.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/handler/deployment.goplatform-api/src/internal/model/gateway_event.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/dto/api.goplatform-api/src/internal/model/api.goplatform-api/src/internal/repository/api.goplatform-api/src/internal/service/deployment.go
📚 Learning: 2026-01-09T05:58:05.318Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/memory.go:484-503
Timestamp: 2026-01-09T05:58:05.318Z
Learning: In gateway/gateway-controller/pkg/storage/memory.go, during API key rotation via the StoreAPIKey method, the API key ID is never modified. When an existing API key is found by name (existingKeyID != ""), the incoming apiKey.ID is guaranteed to be the same as existingKeyID, so no ID mismatch can occur during updates.
Applied to files:
platform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/service/gateway_internal.go
📚 Learning: 2026-01-09T05:57:36.635Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 644
File: gateway/gateway-controller/pkg/storage/apikey_store.go:219-222
Timestamp: 2026-01-09T05:57:36.635Z
Learning: In gateway/gateway-controller/pkg/storage/apikey_store.go, during API key rotation, the API key ID and name remain constant—only the API key value changes. Therefore, when addToAPIMapping finds an existing key by name, the existingKeyID will always equal the incoming apiKey.ID.
Applied to files:
platform-api/src/internal/handler/gateway_internal.go
📚 Learning: 2025-12-18T08:33:51.340Z
Learnt from: O-sura
Repo: wso2/api-platform PR: 485
File: gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go:21-90
Timestamp: 2025-12-18T08:33:51.340Z
Learning: In gateway/policy-engine/internal/analytics/dto/unwrappedEvent.go, the JSON tag casing differences between DefaultFaultEvent (correlationID, applicationID, organizationID, environmentID, regionID with uppercase 'ID') and DefaultResponseEvent (correlationId, applicationId, organizationId, environmentId, regionId with lowercase 'Id') are intentional to maintain consistency with respective event type conventions and config file property standards.
Applied to files:
platform-api/src/internal/model/gateway_event.go
📚 Learning: 2026-01-07T17:41:01.083Z
Learnt from: VirajSalaka
Repo: wso2/api-platform PR: 642
File: gateway/gateway-controller/pkg/utils/api_deployment.go:570-589
Timestamp: 2026-01-07T17:41:01.083Z
Learning: In gateway/gateway-controller/pkg/utils/api_deployment.go, when enableReplicaSync is true (multi-replica mode), the in-memory store (s.store) is intentionally not updated immediately during DeployAPIConfiguration. Instead, all replicas (including the originating replica) update their in-memory stores through the event-driven path via the EventListener to ensure consistent state synchronization across the cluster. This means immediate reads may not find the config until the EventListener processes the event, which is expected behavior.
Applied to files:
platform-api/src/internal/service/gateway_internal.go
📚 Learning: 2025-11-19T06:49:01.751Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 162
File: platform-api/src/resources/openapi.yaml:299-343
Timestamp: 2025-11-19T06:49:01.751Z
Learning: In OpenAPI 3.1 specification, the `contentType` field within the `encoding` object (used for multipart/form-data) can be a comma-separated list of media types. This is valid per the OpenAPI 3.1 Encoding Object specification and should not be flagged as an error.
Applied to files:
platform-api/src/resources/openapi.yaml
🧬 Code graph analysis (7)
platform-api/src/internal/service/gateway_events.go (4)
platform-api/src/internal/model/gateway_event.go (1)
APIUndeploymentEvent(66-75)platform-api/src/internal/dto/gateway_event.go (1)
GatewayEventDTO(21-33)gateway/gateway-controller/pkg/api/generated/generated.go (1)
Payload(95-95)platform-api/src/internal/websocket/stats.go (1)
DeliveryStats(31-51)
platform-api/src/internal/handler/gateway_internal.go (2)
platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/handler/deployment.go (6)
platform-api/src/internal/service/deployment.go (1)
DeploymentService(37-45)platform-api/src/internal/middleware/auth.go (1)
GetOrganizationFromContext(169-176)platform-api/src/internal/utils/error.go (1)
NewErrorResponse(28-37)platform-api/src/internal/dto/api.go (1)
DeployAPIRequest(166-170)platform-api/src/internal/constants/error.go (3)
ErrAPINotFound(47-47)ErrGatewayNotFound(62-62)ErrDeploymentNotFound(68-68)gateway/gateway-controller/pkg/api/generated/generated.go (3)
POST(227-227)GET(225-225)DELETE(224-224)
platform-api/src/internal/server/server.go (2)
platform-api/src/internal/service/deployment.go (1)
NewDeploymentService(48-66)platform-api/src/internal/handler/deployment.go (1)
NewDeploymentHandler(38-42)
platform-api/src/internal/service/gateway_internal.go (1)
platform-api/src/internal/constants/error.go (1)
ErrDeploymentNotActive(69-69)
platform-api/src/internal/dto/api.go (1)
platform-api/src/internal/dto/common.go (1)
Pagination(23-27)
platform-api/src/internal/repository/api.go (1)
platform-api/src/internal/model/api.go (2)
APIDeployment(243-253)APIDeployment(256-258)
🔇 Additional comments (23)
platform-api/src/config/config.go (2)
48-49: LGTM!The new
Deploymentsfield follows the established pattern for nested configuration structs in this file, with proper envconfig tagging.
111-114: LGTM!The
Deploymentsstruct is well-defined with a sensible default value. The envconfig tag follows the established naming convention.platform-api/src/internal/model/gateway_event.go (1)
48-62: LGTM!The
APIDeploymentEventstruct changes align well with the PR's transition from revision-based to deployment-based architecture. TheDeploymentIDfield with itsdeploymentIdJSON tag maintains consistency with the existing naming conventions in this file.platform-api/src/internal/constants/error.go (1)
67-70: LGTM!The new deployment error constants follow the established naming convention and provide clear, descriptive error messages. Grouping them in a separate
varblock maintains consistency with other domain-specific error groups in this file.platform-api/src/internal/database/schema.sql (1)
222-234: Verify intent: UNIQUE constraint may be redundant with PRIMARY KEY.Since
deployment_idis already the PRIMARY KEY (globally unique), theUNIQUE(api_uuid, gateway_uuid, deployment_id, organization_uuid)constraint provides no additional uniqueness enforcement—any combination includingdeployment_idwill already be unique.If the intent is to create a composite index for query performance, consider using an explicit index instead. If the intent is to enforce that each
(api_uuid, gateway_uuid)pair can only have one deployment, the constraint should excludedeployment_id:UNIQUE(api_uuid, gateway_uuid, organization_uuid)platform-api/src/internal/handler/gateway_internal.go (1)
124-138: Active‑deployment lookup path looks good.
The gateway-scoped lookup and explicit 404 for “no active deployment” are clean and consistent.platform-api/src/internal/server/server.go (1)
104-146: Deployment wiring is clean.
Service construction and handler registration align with the new deployment API surface.platform-api/src/internal/model/api.go (1)
242-252: > Likely an incorrect or invalid review comment.platform-api/src/resources/openapi.yaml (6)
792-826: Deploy endpoint definition looks consistent.
Request/response structure aligns with the new deployment flow.
828-855: Deployment list endpoint looks good.
856-908: Deployment get/delete endpoints look consistent.
3675-3768: Deployment schema additions look fine.
3971-3999: New deployment parameters are clear.
4009-4010: Tag update looks good.platform-api/src/internal/repository/interfaces.go (1)
57-67: APIRepository deployment interface additions look good.platform-api/src/internal/repository/api.go (2)
64-75: Expanded API queries and deployment count helper look consistent.Also applies to: 124-135, 180-200, 229-266, 366-374, 1408-1422
996-1068: Deployment retrieval helpers look good.Also applies to: 1070-1106, 1171-1208, 1210-1247
platform-api/src/internal/service/deployment.go (6)
36-65: DeploymentService wiring looks good.
68-95: Core deploy flow (validation/content generation/event broadcast) looks good.Also applies to: 114-153, 155-199, 206-229
232-238: Redeploy/undeploy/delete/read flows look consistent.Also applies to: 242-251, 259-295, 297-303, 307-335, 338-344, 348-358, 402-417, 421-432, 434-440, 444-451
360-399: Deployment listing DTO construction looks good.
454-479: Association helper logic looks fine.
482-573: Handle-based wrappers look consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| // DeploymentResponse represents a deployment artifact | ||
| type DeploymentResponse struct { | ||
| DeploymentID string `json:"deploymentId" yaml:"deploymentId"` | ||
| ApiID string `json:"apiId" yaml:"apiId"` | ||
| GatewayID string `json:"gatewayId" yaml:"gatewayId"` | ||
| Status string `json:"status" yaml:"status"` | ||
| BaseDeploymentID *string `json:"baseDeploymentId,omitempty" yaml:"baseDeploymentId,omitempty"` | ||
| Metadata map[string]interface{} `json:"metadata,omitempty" yaml:"metadata,omitempty"` | ||
| CreatedAt time.Time `json:"createdAt" yaml:"createdAt"` | ||
| } | ||
|
|
||
| // DeploymentListResponse represents a list of deployments | ||
| type DeploymentListResponse struct { | ||
| Count int `json:"count" yaml:"count"` | ||
| List []*DeploymentResponse `json:"list" yaml:"list"` | ||
| Pagination Pagination `json:"pagination" yaml:"pagination"` | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align deployment DTOs with the OpenAPI contract.
OpenAPI defines DeploymentResponse.organizationId and DeploymentListResponse.deployments (no pagination), but DTOs omit OrganizationID and use list + pagination. This will serialize responses that don’t conform to the published schema. Update DTOs/handlers or adjust OpenAPI to match the actual response shape.
🔧 One possible alignment (DTO matches OpenAPI)
type DeploymentResponse struct {
DeploymentID string `json:"deploymentId" yaml:"deploymentId"`
ApiID string `json:"apiId" yaml:"apiId"`
+ OrganizationID string `json:"organizationId" yaml:"organizationId"`
GatewayID string `json:"gatewayId" yaml:"gatewayId"`
Status string `json:"status" yaml:"status"`
BaseDeploymentID *string `json:"baseDeploymentId,omitempty" yaml:"baseDeploymentId,omitempty"`
Metadata map[string]interface{} `json:"metadata,omitempty" yaml:"metadata,omitempty"`
CreatedAt time.Time `json:"createdAt" yaml:"createdAt"`
}
type DeploymentListResponse struct {
Count int `json:"count" yaml:"count"`
- List []*DeploymentResponse `json:"list" yaml:"list"`
- Pagination Pagination `json:"pagination" yaml:"pagination"`
+ Deployments []*DeploymentResponse `json:"deployments" yaml:"deployments"`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // DeploymentResponse represents a deployment artifact | |
| type DeploymentResponse struct { | |
| DeploymentID string `json:"deploymentId" yaml:"deploymentId"` | |
| ApiID string `json:"apiId" yaml:"apiId"` | |
| GatewayID string `json:"gatewayId" yaml:"gatewayId"` | |
| Status string `json:"status" yaml:"status"` | |
| BaseDeploymentID *string `json:"baseDeploymentId,omitempty" yaml:"baseDeploymentId,omitempty"` | |
| Metadata map[string]interface{} `json:"metadata,omitempty" yaml:"metadata,omitempty"` | |
| CreatedAt time.Time `json:"createdAt" yaml:"createdAt"` | |
| } | |
| // DeploymentListResponse represents a list of deployments | |
| type DeploymentListResponse struct { | |
| Count int `json:"count" yaml:"count"` | |
| List []*DeploymentResponse `json:"list" yaml:"list"` | |
| Pagination Pagination `json:"pagination" yaml:"pagination"` | |
| } | |
| // DeploymentResponse represents a deployment artifact | |
| type DeploymentResponse struct { | |
| DeploymentID string `json:"deploymentId" yaml:"deploymentId"` | |
| ApiID string `json:"apiId" yaml:"apiId"` | |
| OrganizationID string `json:"organizationId" yaml:"organizationId"` | |
| GatewayID string `json:"gatewayId" yaml:"gatewayId"` | |
| Status string `json:"status" yaml:"status"` | |
| BaseDeploymentID *string `json:"baseDeploymentId,omitempty" yaml:"baseDeploymentId,omitempty"` | |
| Metadata map[string]interface{} `json:"metadata,omitempty" yaml:"metadata,omitempty"` | |
| CreatedAt time.Time `json:"createdAt" yaml:"createdAt"` | |
| } | |
| // DeploymentListResponse represents a list of deployments | |
| type DeploymentListResponse struct { | |
| Count int `json:"count" yaml:"count"` | |
| Deployments []*DeploymentResponse `json:"deployments" yaml:"deployments"` | |
| } |
🤖 Prompt for AI Agents
In `@platform-api/src/internal/dto/api.go` around lines 172 - 188, Update the DTOs
to match the OpenAPI contract: add an OrganizationID string field to
DeploymentResponse (json:"organizationId" yaml:"organizationId"), and change
DeploymentListResponse to expose Deployments []*DeploymentResponse
`json:"deployments" yaml:"deployments"` instead of Count/List/Pagination (i.e.,
remove Count and Pagination and rename List to Deployments). Also update any
code that constructs or consumes DeploymentResponse and DeploymentListResponse
(handlers/serializers) to populate OrganizationID and to produce the Deployments
slice (and stop expecting Count/Pagination).
| // Ensure API-Gateway association exists | ||
| if err := s.ensureAPIGatewayAssociation(apiUUID, req.GatewayID, orgUUID); err != nil { | ||
| log.Printf("[WARN] Failed to ensure API-gateway association: %v", err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t ignore API–gateway association failures.
If association creation fails, returning success leaves a deployed artifact without a gateway association. Treat this as fatal (or rollback) so deployment and association stay consistent. Based on learnings, deployments are expected to auto-create gateway associations when missing.
🤖 Prompt for AI Agents
In `@platform-api/src/internal/service/deployment.go` around lines 201 - 204, The
code currently swallows failures from s.ensureAPIGatewayAssociation(apiUUID,
req.GatewayID, orgUUID) by logging a warning; instead treat association failures
as fatal: propagate the error (or wrap it with context) and return it from the
enclosing function so the deployment is aborted/rolled back rather than reported
as successful. Update the surrounding control flow to return the error from the
caller (or trigger the existing rollback path) and include contextual
information (apiUUID, req.GatewayID, orgUUID) in the returned/wrapped error to
aid debugging.
| // GetDeploymentContentByHandle retrieves deployment artifact content using API handle | ||
| func (s *DeploymentService) GetDeploymentContentByHandle(apiHandle, deploymentID, orgUUID string) ([]byte, error) { | ||
| // Convert API handle to UUID | ||
| apiUUID, err := s.getAPIUUIDByHandle(apiHandle, orgUUID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return s.GetDeploymentContent(deploymentID, apiUUID, orgUUID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix argument order in GetDeploymentContentByHandle.
The call swaps apiUUID and deploymentID, which will always miss or return incorrect content.
🐛 Proposed fix
- return s.GetDeploymentContent(deploymentID, apiUUID, orgUUID)
+ return s.GetDeploymentContent(apiUUID, deploymentID, orgUUID)🤖 Prompt for AI Agents
In `@platform-api/src/internal/service/deployment.go` around lines 575 - 583,
GetDeploymentContentByHandle builds apiUUID via getAPIUUIDByHandle but then
calls GetDeploymentContent with the arguments swapped; change the call in
GetDeploymentContentByHandle to pass deploymentID, apiUUID, orgUUID (instead of
apiUUID, deploymentID, orgUUID) so GetDeploymentContent receives (deploymentID,
apiUUID, orgUUID) in the correct order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements a new deployment feature that replaces the previous revision-based deployment system with an immutable deployment artifact architecture. The change enables better deployment lifecycle management with versioned, immutable snapshots of API configurations.
Changes:
- Introduces immutable deployment artifacts with lifecycle states (DEPLOYED/UNDEPLOYED)
- Removes API revision fields and endpoints in favor of deployment-centric operations
- Adds comprehensive deployment management endpoints (deploy, redeploy, undeploy, delete, retrieve content)
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| platform-api/src/resources/openapi.yaml | Replaces revision-based deployment endpoints with new artifact-based deployment API; adds deployment schemas and removes revision schemas |
| platform-api/src/internal/service/deployment.go | New service implementing complete deployment lifecycle with artifact creation, status management, and gateway event broadcasting |
| platform-api/src/internal/handler/deployment.go | New handler exposing deployment REST endpoints with validation and error handling |
| platform-api/src/internal/repository/api.go | Updates deployment repository methods to support artifact storage, filtering, and status management |
| platform-api/src/internal/repository/interfaces.go | Extends APIRepository interface with comprehensive deployment artifact methods |
| platform-api/src/internal/service/api.go | Removes revision deployment logic and related request structures |
| platform-api/src/internal/service/gateway_internal.go | Adds active deployment retrieval by gateway for runtime consumption |
| platform-api/src/internal/service/gateway_events.go | Adds undeployment event broadcasting capability |
| platform-api/src/internal/handler/api.go | Removes revision deployment endpoint handler |
| platform-api/src/internal/handler/gateway_internal.go | Updates GetAPI to fetch active deployment instead of current API state |
| platform-api/src/internal/utils/api.go | Removes revision-related fields from DTO/Model conversions |
| platform-api/src/internal/model/api.go | Updates API and APIDeployment models to remove revision fields and add artifact fields |
| platform-api/src/internal/model/gateway_event.go | Changes deployment event to reference deploymentId instead of revisionId |
| platform-api/src/internal/dto/api.go | Replaces APIRevisionDeployment with new deployment request/response DTOs |
| platform-api/src/internal/database/schema.sql | Removes revision columns from apis table; transforms api_deployments into artifact storage table |
| platform-api/src/internal/constants/error.go | Adds deployment-specific error constants |
| platform-api/src/internal/server/server.go | Registers new deployment service and handler |
| platform-api/src/config/config.go | Adds deployment configuration for artifact limits per API-gateway pair |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| log.Printf("[ERROR] Failed to get deployment content: apiId=%s deploymentId=%s error=%v", apiId, deploymentId, err) | ||
| c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error", | ||
| "Failed to retrieve deployment content")) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing return statement after error handling. If an error occurs, the function continues to execute line 354, which will attempt to use an uninitialized 'content' variable. Add 'return' after line 352.
| "Failed to retrieve deployment content")) | |
| "Failed to retrieve deployment content")) | |
| return |
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusNoContent, nil) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the OpenAPI specification at line 960, the undeploy endpoint should return a 200 status with a DeploymentResponse body, not 204 No Content. This inconsistency between the implementation and the API specification will cause issues for API consumers. Update to return StatusOK with the updated deployment response.
| c.JSON(http.StatusNoContent, nil) | |
| deploymentResponse := dto.DeploymentResponse{} | |
| c.JSON(http.StatusOK, deploymentResponse) |
| return nil, err | ||
| } | ||
|
|
||
| return s.GetDeploymentContent(deploymentID, apiUUID, orgUUID) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect parameter order in GetDeploymentContent call. The method signature expects (apiUUID, deploymentID, orgUUID) but this passes (deploymentID, apiUUID, orgUUID). This will cause the wrong data to be queried.
| return s.GetDeploymentContent(deploymentID, apiUUID, orgUUID) | |
| return s.GetDeploymentContent(apiUUID, deploymentID, orgUUID) |
| return nil, err | ||
| } | ||
| if deployment == nil { | ||
| return nil, errors.New("deployment not found") |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the pre-defined constant 'constants.ErrDeploymentNotFound' for consistency with error handling throughout the codebase, rather than creating a new error string.
| return nil, errors.New("deployment not found") | |
| return nil, constants.ErrDeploymentNotFound |
| return nil, err | ||
| } | ||
| if deployment == nil { | ||
| return nil, errors.New("deployment not found") |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use the pre-defined constant 'constants.ErrDeploymentNotFound' for consistency with error handling throughout the codebase, rather than creating a new error string.
| ad.created_at as deployed_at | ||
| FROM gateways g | ||
| INNER JOIN api_associations aa ON g.uuid = aa.resource_uuid AND association_type = 'gateway' | ||
| INNER JOIN api_associations aa ON g.uuid = aa.resource_uuid AND aa.association_type = 'gateway' |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The WHERE clause condition 'aa.association_type = 'gateway'' should use a constant or parameter binding instead of a hardcoded string. Additionally, this condition should be in the JOIN's ON clause for correctness. The association_type check belongs in the WHERE clause of the outer query, not the JOIN condition, or should reference constants.AssociationTypeGateway value.
| return | ||
| } | ||
|
|
||
| c.JSON(http.StatusNoContent, nil) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the OpenAPI specification at line 894, the delete endpoint returns 204 No Content on success. However, OpenAPI also defines a 409 Conflict response for when a deployment is currently DEPLOYED and cannot be deleted. The handler should check the error type and return a 409 status with appropriate error message when deletion fails due to deployment status conflicts.
Purpose
Goals
Approach
User stories
Documentation
Automation tests
Security checks
Samples
Related PRs
Test environment
Summary by CodeRabbit
Release Notes
New Features
Revert
✏️ Tip: You can customize this high-level summary in your review settings.